Re: CONFIG_DEBUG_SHIRQ and PM
On 26 August 2015 at 17:03, Felipe Balbi ba...@ti.com wrote:
On Wed, Aug 26, 2015 at 04:53:27PM -0300, Ezequiel Garcia wrote:
On 26 August 2015 at 16:38, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Aug 26, 2015 at 04:29:52PM -0300, Ezequiel Garcia wrote:
Felipe,
On 25 August 2015 at 16:58, Felipe Balbi ba...@ti.com wrote:
Hi Ingo,
I'm facing an issue with CONFIG_DEBUG_SHIRQ and pm_runtime when using
devm_request_*irq().
I may be jumping on the gun here, but I believe here's your problem.
Using devm_request_irq with shared IRQs is not a good idea.
Or at least, it forces you to handle interrupts after your device
is _completely_ removed (e.g. your IRQ cookie could be bogus).
AFAICS, the CONFIG_DEBUG_SHIRQ option is just triggering a couple
spurious interrupts, that are expected to happen anyway.
Your IRQ is shared, and so you may get any IRQ at any time,
coming from another device (not yours).
So, if I'm right, my suggestion is simple: use request_irq/free_irq
and free your IRQ before you disable your clocks, remove your device,
etc.
yeah, that's just a workaround though. Specially with IRQ flags coming
from DT, driver might have no knowledge that its IRQ is shared to start
with.
Really? Is there any way the DT can set the IRQF_SHARED flag?
The caller of devm_request_irq / request_irq needs to pass the irqflags,
so I'd say the driver is perfectly aware of the IRQ being shared or not.
Maybe you can clarify what I'm missing here.
hmm, that's true. Now that I look at it, DT can pass triggering flags.
Besides, removing devm_* is just a workaround to the real problem. It
seems, to me at least, that drivers shouldn't be calling
pm_runtime_put_sync(); pm_runtime_disable() from their -remove(),
rather the bus driver should be responsible for doing so; much
usb_bus_type and pci_bus_type do. Of course, this has the side effect of
requiring buses to make sure that by the time -probe() is called, that
device's clocks are stable and running and the device is active.
However, that's not done today for the platform_bus_type and, frankly,
that would be somewhat of a complex problem to solve, considering that
different SoCs integrate IPs the way they want.
Personally, I think removing devm_* is but a workaround to the real
thing we're dealing with.
I don't see any problems here: if your interrupt is shared, then you must
be prepared to handle it any time, coming from any sources (not only
your device). And CONFIG_DEBUG_SHIRQ does exactly that, in order to
make sure all the drivers passing IRQF_SHARED comply with that rule.
you need to be sure of that with non-shared IRQs anyway.
Not entirely. If your IRQ is not shared, then you usually have a register
to enable or unmask your peripheral interrupts. So the driver is in control
of when it will get interrupts.
If the IRQ is shared, this won't do. This is what I mean by shared IRQs
must be prepared to receive an interrupt any time, in the sense that
the driver has no way of preventing IRQs (because they may be
coming from any source).
In the same sense, shared IRQs handlers need to double-check
the IRQ is coming to the current device by checking some IRQ
status register to see if there's pending work.
Also, an IRQ
which isn't shared in SoC A, might become shared in SoC B which uses the
same IP.
So you either avoid using devm_request_irq, or you prepare your handler
accordingly to be ready to handle an interrupt _any time_.
the handler is ready to handle at any time, what isn't correct is the
fact that clocks get gated before IRQ is freed.
There should be no such special case as if your handler is shared,
don't use devm_request_*irq() because if we just disable PM_RUNTIME, it
works as expected anyway.
Yeah, I meant to say: if you use devm_request_irq with IRQF_SHARED
then you _must_ be prepared to get an IRQ *after* your remove() has
been called.
Let's consider this snippet from tw68:
static irqreturn_t tw68_irq(int irq, void *dev_id)
{
struct tw68_dev *dev = dev_id;
u32 status, orig;
int loop;
status = orig = tw_readl(TW68_INTSTAT) dev-pci_irqmask;
[etc]
}
The IRQ handler accesses the device struct and then
reads through PCI. So if you use devm_request_irq
you need to make sure the device struct is still allocated
after remove(), and the PCI read won't stall or crash.
Interestingly, tw68 uses devm_request_irq with IRQF_SHARED :-)
Still, I don't think that's a good idea, since it relies on
the IRQ being freed *before* the device struct.
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
* Guenter Roeck li...@roeck-us.net [150826 11:37]:
On 08/26/2015 10:04 AM, Tony Lindgren wrote:
Hi,
* Guenter Roeck li...@roeck-us.net [150817 13:48]:
Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes
the call to smsc911x_probe_config() unconditional, and no longer fails if
there is no device node. device_get_phy_mode() is called unconditionally,
and if there is no phy node configured returns an error code. This error
code is assigned to phy_interface, and interpreted elsewhere in the code
as valid phy mode. This in turn causes qemu to crash when running a
variant of realview_pb_defconfig.
qemu: hardware error: lan9118_read: Bad reg 0x86
Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT)
Cc: Jeremy Linton jeremy.lin...@arm.com
Cc Graeme Gregory graeme.greg...@linaro.org
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/smsc/smsc911x.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c
b/drivers/net/ethernet/smsc/smsc911x.c
index 0f21aa3bb537..34f97684506b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops
shifted_smsc911x_ops = {
static int smsc911x_probe_config(struct smsc911x_platform_config *config,
struct device *dev)
{
+ int phy_interface;
u32 width = 0;
if (!dev)
return -ENODEV;
- config-phy_interface = device_get_phy_mode(dev);
+ phy_interface = device_get_phy_mode(dev);
+ if (phy_interface 0)
+ return phy_interface;
+
+ config-phy_interface = phy_interface;
device_get_mac_address(dev, config-mac, ETH_ALEN);
Looks like this change makes at least omap boards using smsc911x
fail with -22 for me in Linux next.
Do any of the the device tree configured smsc911x devices actually
have a phy configured?
Ok, this is more subtle than I thought.
Previously, the code would not attempt any devicetree configuration
if devicetree was not configured.
Now it does.
The error return from device_get_phy_mode() isn't the actual problem.
Apparently it doesn't really matter if a nonsensical value is assigned
to phy_interface.
The problem is that the reg-io-width property is obviously not present
in the non-dt and non-acpi case. This overwrites the existing platform data
configuration and selects 16 bit mode, to which the (simulated) hardware
obviously reacts less than enthusiastic.
Fixing this properly won't be easy. If the reg-io-width property
is not present or wrong, the default register width is 16 bit. Obviously,
if neither DT nor ACPI is available, it won't be present. This causes
the crash I had observed.
Heh OK :)
Bad part is that there does not seem to be a reliable means to detect
that platform data should be used in that situation. Other device_get_XXX
functions return -ENXIO if that happens, but not device_property_read_u32().
It is _supposed_ to return it per its API, but it doesn't (it returns
-ENODATA).
We may need two separate patches, one to fix up device_property_read_u32()
to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error
from device_get_phy_mode(), and to bail out if device_property_read_u32()
returns -ENXIO.
I guess the device_property_read_u32() change needs to be discussed
separately.. So probably best to fix up the regression to smsc911x
first.
The simpler alternative would be to check the return value from
device_property_read_u32() for both -ENXIO and -ENODATA.
This would make the code independent of the necessary core changes
(which may take a while). I tested this variant, and it works, at least
for the non-DT case.
Does this make sense ?
Yeh I think that would allow fixing up the smsc911x regression while
discussing the device_property_read_u32() change. Got a test patch
for me to try?
Regards,
Tony
--
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: CONFIG_DEBUG_SHIRQ and PM
Felipe, On 25 August 2015 at 16:58, Felipe Balbi ba...@ti.com wrote: Hi Ingo, I'm facing an issue with CONFIG_DEBUG_SHIRQ and pm_runtime when using devm_request_*irq(). I may be jumping on the gun here, but I believe here's your problem. Using devm_request_irq with shared IRQs is not a good idea. Or at least, it forces you to handle interrupts after your device is _completely_ removed (e.g. your IRQ cookie could be bogus). AFAICS, the CONFIG_DEBUG_SHIRQ option is just triggering a couple spurious interrupts, that are expected to happen anyway. Your IRQ is shared, and so you may get any IRQ at any time, coming from another device (not yours). So, if I'm right, my suggestion is simple: use request_irq/free_irq and free your IRQ before you disable your clocks, remove your device, etc. If we using devm_request_*irq(), that irq will be freed after device drivers' -remove() gets called. If on -remove(), we're calling pm_runtime_put_sync(); pm_runtime_disable(), device's clocks might get gated and, because we do an extra call to the device's IRQ handler when CONFIG_DEBUG_SHIRQ=y, we might trigger an abort exception if, inside the IRQ handler, we try to read a register which is clocked by the device's clock. This is, of course, really old code which has been in tree for many, many years. I guess nobody has been running their tests in the setup mentioned above (CONFIG_DEBUG_SHIRQ=y, pm_runtime_put_sync() on -remove(), a register read on IRQ handler, and a shared IRQ handler), so that's why we never caught this before. Disabling CONFIG_DEBUG_SHIRQ, of course, makes the problem go away, but if driver *must* be ready to receive, and handle, an IRQ even during module removal, I wonder what the IRQ handler should do. We can't, in most cases, call pm_runtime_put_sync() from IRQ handler. I'm guessing the only way would be to move pm_runtime calls into the bus driver (in this case, the platform_bus) and make sure it only gets called after device managed resources are freed ? Any hints would be greatly appreciated. Hope it helps! -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- 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: CONFIG_DEBUG_SHIRQ and PM
Hi, On Wed, Aug 26, 2015 at 04:29:52PM -0300, Ezequiel Garcia wrote: Felipe, On 25 August 2015 at 16:58, Felipe Balbi ba...@ti.com wrote: Hi Ingo, I'm facing an issue with CONFIG_DEBUG_SHIRQ and pm_runtime when using devm_request_*irq(). I may be jumping on the gun here, but I believe here's your problem. Using devm_request_irq with shared IRQs is not a good idea. Or at least, it forces you to handle interrupts after your device is _completely_ removed (e.g. your IRQ cookie could be bogus). AFAICS, the CONFIG_DEBUG_SHIRQ option is just triggering a couple spurious interrupts, that are expected to happen anyway. Your IRQ is shared, and so you may get any IRQ at any time, coming from another device (not yours). So, if I'm right, my suggestion is simple: use request_irq/free_irq and free your IRQ before you disable your clocks, remove your device, etc. yeah, that's just a workaround though. Specially with IRQ flags coming from DT, driver might have no knowledge that its IRQ is shared to start with. Besides, removing devm_* is just a workaround to the real problem. It seems, to me at least, that drivers shouldn't be calling pm_runtime_put_sync(); pm_runtime_disable() from their -remove(), rather the bus driver should be responsible for doing so; much usb_bus_type and pci_bus_type do. Of course, this has the side effect of requiring buses to make sure that by the time -probe() is called, that device's clocks are stable and running and the device is active. However, that's not done today for the platform_bus_type and, frankly, that would be somewhat of a complex problem to solve, considering that different SoCs integrate IPs the way they want. Personally, I think removing devm_* is but a workaround to the real thing we're dealing with. -- balbi signature.asc Description: Digital signature
Re: CONFIG_DEBUG_SHIRQ and PM
On Wed, Aug 26, 2015 at 04:53:27PM -0300, Ezequiel Garcia wrote: On 26 August 2015 at 16:38, Felipe Balbi ba...@ti.com wrote: Hi, On Wed, Aug 26, 2015 at 04:29:52PM -0300, Ezequiel Garcia wrote: Felipe, On 25 August 2015 at 16:58, Felipe Balbi ba...@ti.com wrote: Hi Ingo, I'm facing an issue with CONFIG_DEBUG_SHIRQ and pm_runtime when using devm_request_*irq(). I may be jumping on the gun here, but I believe here's your problem. Using devm_request_irq with shared IRQs is not a good idea. Or at least, it forces you to handle interrupts after your device is _completely_ removed (e.g. your IRQ cookie could be bogus). AFAICS, the CONFIG_DEBUG_SHIRQ option is just triggering a couple spurious interrupts, that are expected to happen anyway. Your IRQ is shared, and so you may get any IRQ at any time, coming from another device (not yours). So, if I'm right, my suggestion is simple: use request_irq/free_irq and free your IRQ before you disable your clocks, remove your device, etc. yeah, that's just a workaround though. Specially with IRQ flags coming from DT, driver might have no knowledge that its IRQ is shared to start with. Really? Is there any way the DT can set the IRQF_SHARED flag? The caller of devm_request_irq / request_irq needs to pass the irqflags, so I'd say the driver is perfectly aware of the IRQ being shared or not. Maybe you can clarify what I'm missing here. hmm, that's true. Now that I look at it, DT can pass triggering flags. Besides, removing devm_* is just a workaround to the real problem. It seems, to me at least, that drivers shouldn't be calling pm_runtime_put_sync(); pm_runtime_disable() from their -remove(), rather the bus driver should be responsible for doing so; much usb_bus_type and pci_bus_type do. Of course, this has the side effect of requiring buses to make sure that by the time -probe() is called, that device's clocks are stable and running and the device is active. However, that's not done today for the platform_bus_type and, frankly, that would be somewhat of a complex problem to solve, considering that different SoCs integrate IPs the way they want. Personally, I think removing devm_* is but a workaround to the real thing we're dealing with. I don't see any problems here: if your interrupt is shared, then you must be prepared to handle it any time, coming from any sources (not only your device). And CONFIG_DEBUG_SHIRQ does exactly that, in order to make sure all the drivers passing IRQF_SHARED comply with that rule. you need to be sure of that with non-shared IRQs anyway. Also, an IRQ which isn't shared in SoC A, might become shared in SoC B which uses the same IP. So you either avoid using devm_request_irq, or you prepare your handler accordingly to be ready to handle an interrupt _any time_. the handler is ready to handle at any time, what isn't correct is the fact that clocks get gated before IRQ is freed. There should be no such special case as if your handler is shared, don't use devm_request_*irq() because if we just disable PM_RUNTIME, it works as expected anyway. -- balbi signature.asc Description: Digital signature
Re: [PATCH] ARM: OMAP2+: omap-device: remove omap_device_late_init call completely
* Grygorii Strashko grygorii.stras...@ti.com [150826 11:01]:
Now Kernel fails to boot 50% of times (form build to build) with
RT-patchset applied due to the following race - on late boot
stages deferred_probe_work_func races with omap_device_late_ini
late_initcall
- deferred_probe_initcal() tries to re-probe all pending driver's probe.
[In general, It's NOT expected to probe any other built-in drivers after
deferred_probe_initcal() is finished, because most of
late_initcall_sync/late_initcall functions expected that all driver
or probed or deferred already.]
- later on, some driver is probing in this case It's could cpsw.c
(but could be any other drivers)
cpsw_init
- platform_driver_register
- really_probe
- driver_bound
- driver_deferred_probe_trigger
and boot proceed.
So, at this moment we have deferred_probe_work_func scheduled.
late_initcall_sync
- omap_device_late_init
- omap_device_idle
CPU1 CPU2
- deferred_probe_work_func
- really_probe
- omap_hsmmc_probe
- pm_runtime_get_sync
late_initcall_sync
- omap_device_late_init
if (od-_driver_status !=
BUS_NOTIFY_BOUND_DRIVER) {
if (od-_state ==
OMAP_DEVICE_STATE_ENABLED) {
-
omap_device_idle [ops - IP is disabled, ]
- [fail]
- pm_runtime_put_sync
- omap_hsmmc_runtime_suspend [ooops!]
OK idling of unclaimed devices should not happen for deferred probe,
it should only happen when there's no driver and no probing happening.
Lets remove just remove omap_device_late_init completely as suggested
by Tero Kristo:
How about remove omap_device_late_init call completely. I don't think
it does anything useful at the moment; none of the omap devices get
enabled outside runtime_pm, so there should be no need to explicitly
disable the devices.
I think this is still needed from PM point of view as otherwise we
don't idle any devices that don't have a driver available. Or am I
missing something?
To me it seems the bug is relying on the BUS_NOTIFY_BOUND_DRIVER is
not set in the deferred probe case.
Regards,
Tony
--
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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
On 08/26/2015 10:04 AM, Tony Lindgren wrote:
Hi,
* Guenter Roeck li...@roeck-us.net [150817 13:48]:
Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes
the call to smsc911x_probe_config() unconditional, and no longer fails if
there is no device node. device_get_phy_mode() is called unconditionally,
and if there is no phy node configured returns an error code. This error
code is assigned to phy_interface, and interpreted elsewhere in the code
as valid phy mode. This in turn causes qemu to crash when running a
variant of realview_pb_defconfig.
qemu: hardware error: lan9118_read: Bad reg 0x86
Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT)
Cc: Jeremy Linton jeremy.lin...@arm.com
Cc Graeme Gregory graeme.greg...@linaro.org
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/smsc/smsc911x.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c
b/drivers/net/ethernet/smsc/smsc911x.c
index 0f21aa3bb537..34f97684506b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops =
{
static int smsc911x_probe_config(struct smsc911x_platform_config *config,
struct device *dev)
{
+ int phy_interface;
u32 width = 0;
if (!dev)
return -ENODEV;
- config-phy_interface = device_get_phy_mode(dev);
+ phy_interface = device_get_phy_mode(dev);
+ if (phy_interface 0)
+ return phy_interface;
+
+ config-phy_interface = phy_interface;
device_get_mac_address(dev, config-mac, ETH_ALEN);
Looks like this change makes at least omap boards using smsc911x
fail with -22 for me in Linux next.
Do any of the the device tree configured smsc911x devices actually
have a phy configured?
Ok, this is more subtle than I thought.
Previously, the code would not attempt any devicetree configuration
if devicetree was not configured.
Now it does.
The error return from device_get_phy_mode() isn't the actual problem.
Apparently it doesn't really matter if a nonsensical value is assigned
to phy_interface.
The problem is that the reg-io-width property is obviously not present
in the non-dt and non-acpi case. This overwrites the existing platform data
configuration and selects 16 bit mode, to which the (simulated) hardware
obviously reacts less than enthusiastic.
Fixing this properly won't be easy. If the reg-io-width property
is not present or wrong, the default register width is 16 bit. Obviously,
if neither DT nor ACPI is available, it won't be present. This causes
the crash I had observed.
Bad part is that there does not seem to be a reliable means to detect
that platform data should be used in that situation. Other device_get_XXX
functions return -ENXIO if that happens, but not device_property_read_u32().
It is _supposed_ to return it per its API, but it doesn't (it returns
-ENODATA).
We may need two separate patches, one to fix up device_property_read_u32()
to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error
from device_get_phy_mode(), and to bail out if device_property_read_u32()
returns -ENXIO.
The simpler alternative would be to check the return value from
device_property_read_u32() for both -ENXIO and -ENODATA.
This would make the code independent of the necessary core changes
(which may take a while). I tested this variant, and it works, at least
for the non-DT case.
Does this make sense ?
Thanks,
Guenter
--
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
loan services of different types
Fast loan offer for you today at just 2% interest rate, both long and short term loan of all amounts and currencies, no collateral required. Apply now for your instant approval and transfer approval process takes just 24hours. Contact us now through our e-mail E-MAIL osmanlen...@gmail.com -- 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: CONFIG_DEBUG_SHIRQ and PM
On 26 August 2015 at 16:38, Felipe Balbi ba...@ti.com wrote: Hi, On Wed, Aug 26, 2015 at 04:29:52PM -0300, Ezequiel Garcia wrote: Felipe, On 25 August 2015 at 16:58, Felipe Balbi ba...@ti.com wrote: Hi Ingo, I'm facing an issue with CONFIG_DEBUG_SHIRQ and pm_runtime when using devm_request_*irq(). I may be jumping on the gun here, but I believe here's your problem. Using devm_request_irq with shared IRQs is not a good idea. Or at least, it forces you to handle interrupts after your device is _completely_ removed (e.g. your IRQ cookie could be bogus). AFAICS, the CONFIG_DEBUG_SHIRQ option is just triggering a couple spurious interrupts, that are expected to happen anyway. Your IRQ is shared, and so you may get any IRQ at any time, coming from another device (not yours). So, if I'm right, my suggestion is simple: use request_irq/free_irq and free your IRQ before you disable your clocks, remove your device, etc. yeah, that's just a workaround though. Specially with IRQ flags coming from DT, driver might have no knowledge that its IRQ is shared to start with. Really? Is there any way the DT can set the IRQF_SHARED flag? The caller of devm_request_irq / request_irq needs to pass the irqflags, so I'd say the driver is perfectly aware of the IRQ being shared or not. Maybe you can clarify what I'm missing here. Besides, removing devm_* is just a workaround to the real problem. It seems, to me at least, that drivers shouldn't be calling pm_runtime_put_sync(); pm_runtime_disable() from their -remove(), rather the bus driver should be responsible for doing so; much usb_bus_type and pci_bus_type do. Of course, this has the side effect of requiring buses to make sure that by the time -probe() is called, that device's clocks are stable and running and the device is active. However, that's not done today for the platform_bus_type and, frankly, that would be somewhat of a complex problem to solve, considering that different SoCs integrate IPs the way they want. Personally, I think removing devm_* is but a workaround to the real thing we're dealing with. I don't see any problems here: if your interrupt is shared, then you must be prepared to handle it any time, coming from any sources (not only your device). And CONFIG_DEBUG_SHIRQ does exactly that, in order to make sure all the drivers passing IRQF_SHARED comply with that rule. So you either avoid using devm_request_irq, or you prepare your handler accordingly to be ready to handle an interrupt _any time_. -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- 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 v5 0/3] ARM: AM437X: Add rtc clock handling
* Keerthy a0393...@ti.com [150826 09:54]: Tony, On Saturday 22 August 2015 02:48 AM, Alexandre Belloni wrote: Tony, On 18/08/2015 at 15:11:13 +0530, Keerthy wrote : The series is applicable for all am437x series of processors. It adds clock handling support. Boot tested on am437x-gp-evm. Keerthy (3): ARM: dts: AM437x: Add the internal and external clock nodes for rtc rtc: omap: Add internal clock enabling support rtc: omap: Add external clock enabling support I'm wondering how you want to get those patches merged. I can let you take 2 and 3 through arm-soc but you will miss 4.3. Or I can take 2 and 3 for 4.3 but the documentation will be missing. A gentle ping on this series. Alexandre, it's probably best that you take them all. The dts changes apply against Linux next with fuzz so there should not be any merge conflict. Feel free to add: Acked-by: Tony Lindgren t...@atomide.com Regards, Tony -- 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/7] gpio: omap: fixes and improvements
* Grygorii Strashko grygorii.stras...@ti.com [150825 04:44]: On 08/21/2015 11:13 AM, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [150818 23:42]: Hi, * Grygorii Strashko grygorii.stras...@ti.com [150818 04:14]: Hi, This patch series contains set of trivial fixes and improvements, and also patches which fixes wrong APIs usage in atomic context as for -RT as for non-RT kernel. The final goal of this series is to make TI OMAP GPIO driver compatible with -RT kernel as much as possible. Patch 1-4: trivial fixes and improvements Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime can't be used in atimic context on -RT. Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq handler instead of chained IRQ handler. This way OMAP GPIO driver will be compatible with RT kernel where it will be forced thread IRQ handler while in non-RT kernel it still will be executed in HW IRQ context. Based on quick testing this series breaks at least core off idle for omap3. You probably should add a beagle xm to your test devices so you can properly test PM features. Sorry I take that back, after trying to figure out which patch breaks PM I noticed I had some other patches applied also. Looks like PM works just fine with this series for me, so please feel free to add: Uh... :) Thanks Tony a lot and sorry for delayed reply (was ooo). You've restored my heartbeat :) Tested-by: Tony Lindgren t...@atomide.com Note that I have not been able to test this with gpio button as my boards are in a rack. You may want to do some gpio button tests to make sure things wake up properly from off idle if you can get hold of a beagle xm. I posted some instructions how to test earlier today for Kishon in the [PATCH v2 00/16] omap_hsmmc: regulator usage cleanup and fixes thread. I've tested it actually using GPIO buttons and UART console wake-up (Results - https://lkml.org/lkml/2015/8/14/194 :) OK great good to hear :) And I'd try to retest it on beagle xm also, but not sure if i'd be able to test button's wake-up - have shared access only now to beagle xm which is rack also. OK. FYI, we're still missing a clean solution to omap3 errata 1.158 that requires remuxing GPIO pins to safe mode with pull for off idle to avoid glitches: http://www.spinics.net/lists/linux-omap/msg11669.html Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP2+: omap-device: remove omap_device_late_init call completely
Now Kernel fails to boot 50% of times (form build to build) with
RT-patchset applied due to the following race - on late boot
stages deferred_probe_work_func races with omap_device_late_ini
late_initcall
- deferred_probe_initcal() tries to re-probe all pending driver's probe.
[In general, It's NOT expected to probe any other built-in drivers after
deferred_probe_initcal() is finished, because most of
late_initcall_sync/late_initcall functions expected that all driver
or probed or deferred already.]
- later on, some driver is probing in this case It's could cpsw.c
(but could be any other drivers)
cpsw_init
- platform_driver_register
- really_probe
- driver_bound
- driver_deferred_probe_trigger
and boot proceed.
So, at this moment we have deferred_probe_work_func scheduled.
late_initcall_sync
- omap_device_late_init
- omap_device_idle
CPU1CPU2
- deferred_probe_work_func
- really_probe
- omap_hsmmc_probe
- pm_runtime_get_sync
late_initcall_sync
- omap_device_late_init
if (od-_driver_status !=
BUS_NOTIFY_BOUND_DRIVER) {
if (od-_state ==
OMAP_DEVICE_STATE_ENABLED) {
-
omap_device_idle [ops - IP is disabled, ]
- [fail]
- pm_runtime_put_sync
- omap_hsmmc_runtime_suspend [ooops!]
== log ==
omap_hsmmc 480b4000.mmc: unable to get vmmc regulator -517
davinci_mdio 48485000.mdio: davinci mdio revision 1.6
davinci_mdio 48485000.mdio: detected phy mask fff3
libphy: 48485000.mdio: probed
davinci_mdio 48485000.mdio: phy[2]: device 48485000.mdio:02, driver unknown
davinci_mdio 48485000.mdio: phy[3]: device 48485000.mdio:03, driver unknown
omap_hsmmc 480b4000.mmc: unable to get vmmc regulator -517
cpsw 48484000.ethernet: Detected MACID = b4:99:4c:c7:d2:48
cpsw 48484000.ethernet: cpsw: Detected MACID = b4:99:4c:c7:d2:49
hctosys: unable to open rtc device (rtc0)
omap_hsmmc 480b4000.mmc: omap_device_late_idle: enabled but no driver. Idling
ldousb: disabling
Unhandled fault: imprecise external abort (0x1406) at 0x
[] *pgd=
Internal error: : 1406 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 1 PID: 58 Comm: kworker/u4:1 Not tainted 4.1.2-rt1-00467-g6da3c0a-dirty #5
Hardware name: Generic DRA74X (Flattened Device Tree)
Workqueue: deferwq deferred_probe_work_func
task: ee6ddb00 ti: edd3c000 task.ti: edd3c000
PC is at omap_hsmmc_runtime_suspend+0x1c/0x12c
LR is at _od_runtime_suspend+0xc/0x24
pc : [c0471998]lr : [c0029590]psr: a013
sp : edd3dda0 ip : ee6ddb00 fp : c07be540
r10: r9 : c07be540 r8 : 0008
r7 : r6 : ee646c10 r5 : ee646c10 r4 : edd79380
r3 : fa0b4100 r2 : r1 : r0 : ee646c10
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: 8000406a DAC: 0015
Process kworker/u4:1 (pid: 58, stack limit = 0xedd3c218)
Stack: (0xedd3dda0 to 0xedd3e000)
dda0: ee646c70 ee646c10 c0029584 0008 c0029590 ee646c70 ee646c10
ddc0: c0029584 c03adfb8 ee646c10 0004 000c c03adff0 ee646c10 0004
dde0: 000c c03ae4ec edd3c000 ee646c10 0004 ee646c70 0004
de00: fa0b4000 c03aec20 ee6ddb00 ee646c10 0004 ee646c70 ee646c10 fdfb
de20: edd79380 fa0b4000 c03aee90 fdfb edd79000 ee646c00 c0474290
de40: edda24c0 edd79380 edc81f00 0200 0001 c06dd488
de60: edda3960 ee646c10 ee646c10 c0824cc4 fdfb c0880c94 0002 edc92600
de80: c0836378 c03a7f84 ee646c10 c0824cc4 c0880c80 c0880c94 c03a6568
dea0: ee646c10 c03a66ac ee4f8000 0001 edc92600 c03a4b40
dec0: ee404c94 edc83c4c ee646c10 ee646c10 ee646c44 c03a63c4 ee646c10 ee646c10
dee0: c0814448 c03a5aa8 ee646c10 c0814220 edd3c000 c03a5ec0 c0814250 ee6be400
df00: edd3c000 c004e5bc ee6ddb01 0078 ee6ddb00 ee4f8000 ee6be418 edd3c000
df20: ee4f8028 0088 c0836045 ee4f8000 ee6be400 c004e928 ee4f8028
df40: c004e8ec ee6bf1c0 ee6be400 c004e8ec
df60: c0053450 2e56fa97 afdffbd7 ee6be400
df80: edd3df80 edd3df80 edd3df90 edd3df90 edd3dfac ee6bf1c0
dfa0: c0053384 c000f668
dfc0:
dfe0: 0013 f1fc9d7e febfbdff
[c0471998] (omap_hsmmc_runtime_suspend) from [c0029590]
(_od_runtime_suspend+0xc/0x24)
[c0029590] (_od_runtime_suspend) from [c03adfb8] (__rpm_callback+0x24/0x3c)
[c03adfb8] (__rpm_callback) from [c03adff0] (rpm_callback+0x20/0x80)
[c03adff0] (rpm_callback) from
Re: [PATCH -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
On 08/26/2015 12:04 PM, Tony Lindgren wrote: * Guenter Roeck li...@roeck-us.net [150817 13:48]: Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes Looks like this change makes at least omap boards using smsc911x fail with -22 for me in Linux next. Do any of the the device tree configured smsc911x devices actually have a phy configured? Tony, Looks like all the ones in the kernel boot/dts directory have a phy including the omap3-lilly except for the ste-snowball.dts. Do you have smsc,force-internal-phy set instead? Thanks, -- 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/2] OMAPDSS: Couple of HDMI audio fixes
On Wed, Aug 26, 2015 at 04:11:38PM +0300, Jyri Sarha wrote: The two fixes are totally independent and the video and audio side patches applied separately trough their own trees. In general if patches aren't related to each other either through code overlap or through content it's better to just send them as individual changes, that avoids any potential for confusion. signature.asc Description: Digital signature
Re: [PATCH -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
* Jeremy Linton jeremy.lin...@arm.com [150826 10:35]: On 08/26/2015 12:04 PM, Tony Lindgren wrote: * Guenter Roeck li...@roeck-us.net [150817 13:48]: Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes Looks like this change makes at least omap boards using smsc911x fail with -22 for me in Linux next. Do any of the the device tree configured smsc911x devices actually have a phy configured? Tony, Looks like all the ones in the kernel boot/dts directory have a phy including the omap3-lilly except for the ste-snowball.dts. Do you have smsc,force-internal-phy set instead? Hmm most of them are using omap-gpmc-smsc911x.dtsi and omap-gpmc-smsc9221.dtsi which are set up the same way as omap3-lilly. So no phy. Regards, Tony -- 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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
* Guenter Roeck li...@roeck-us.net [150826 10:40]:
Hi Tony,
On 08/26/2015 10:04 AM, Tony Lindgren wrote:
Hi,
* Guenter Roeck li...@roeck-us.net [150817 13:48]:
Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes
the call to smsc911x_probe_config() unconditional, and no longer fails if
there is no device node. device_get_phy_mode() is called unconditionally,
and if there is no phy node configured returns an error code. This error
code is assigned to phy_interface, and interpreted elsewhere in the code
as valid phy mode. This in turn causes qemu to crash when running a
variant of realview_pb_defconfig.
qemu: hardware error: lan9118_read: Bad reg 0x86
Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT)
Cc: Jeremy Linton jeremy.lin...@arm.com
Cc Graeme Gregory graeme.greg...@linaro.org
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/smsc/smsc911x.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c
b/drivers/net/ethernet/smsc/smsc911x.c
index 0f21aa3bb537..34f97684506b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops
shifted_smsc911x_ops = {
static int smsc911x_probe_config(struct smsc911x_platform_config *config,
struct device *dev)
{
+ int phy_interface;
u32 width = 0;
if (!dev)
return -ENODEV;
- config-phy_interface = device_get_phy_mode(dev);
+ phy_interface = device_get_phy_mode(dev);
+ if (phy_interface 0)
+ return phy_interface;
+
+ config-phy_interface = phy_interface;
device_get_mac_address(dev, config-mac, ETH_ALEN);
Looks like this change makes at least omap boards using smsc911x
fail with -22 for me in Linux next.
What do you see if you revert my patch ? It should assign -22, or its
unsigned representation, to phy_interface, which isn't such a good idea
either.
If I revert patch smsc911x: Fix crash seen if neither ACPI nor OF is
configured or used things work as just assign config-phy_interface
directly without returning early. It's -22 in that case also.
Do any of the the device tree configured smsc911x devices actually
have a phy configured?
Good question, and beats me. Looking into the original code,
it didn't check for an error return from of_get_phy_mode() either,
and thus _would_ dutifully assign the error code to phy_interface.
Wonder how was this supposed to work to start with.
I'll do some debugging and try to find out what exactly is going on.
Looks like adding smsc,force-internal-phy to omap-gpmc-smsc9221.dtsi
does not help. Somehow the default behavior is now different.
Regards,
Tony
--
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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
Hi Tony,
On 08/26/2015 10:04 AM, Tony Lindgren wrote:
Hi,
* Guenter Roeck li...@roeck-us.net [150817 13:48]:
Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes
the call to smsc911x_probe_config() unconditional, and no longer fails if
there is no device node. device_get_phy_mode() is called unconditionally,
and if there is no phy node configured returns an error code. This error
code is assigned to phy_interface, and interpreted elsewhere in the code
as valid phy mode. This in turn causes qemu to crash when running a
variant of realview_pb_defconfig.
qemu: hardware error: lan9118_read: Bad reg 0x86
Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT)
Cc: Jeremy Linton jeremy.lin...@arm.com
Cc Graeme Gregory graeme.greg...@linaro.org
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/smsc/smsc911x.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c
b/drivers/net/ethernet/smsc/smsc911x.c
index 0f21aa3bb537..34f97684506b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops =
{
static int smsc911x_probe_config(struct smsc911x_platform_config *config,
struct device *dev)
{
+ int phy_interface;
u32 width = 0;
if (!dev)
return -ENODEV;
- config-phy_interface = device_get_phy_mode(dev);
+ phy_interface = device_get_phy_mode(dev);
+ if (phy_interface 0)
+ return phy_interface;
+
+ config-phy_interface = phy_interface;
device_get_mac_address(dev, config-mac, ETH_ALEN);
Looks like this change makes at least omap boards using smsc911x
fail with -22 for me in Linux next.
What do you see if you revert my patch ? It should assign -22, or its
unsigned representation, to phy_interface, which isn't such a good idea
either.
Do any of the the device tree configured smsc911x devices actually
have a phy configured?
Good question, and beats me. Looking into the original code,
it didn't check for an error return from of_get_phy_mode() either,
and thus _would_ dutifully assign the error code to phy_interface.
Wonder how was this supposed to work to start with.
I'll do some debugging and try to find out what exactly is going on.
Guenter
--
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 v5 0/3] ARM: AM437X: Add rtc clock handling
Tony, On Saturday 22 August 2015 02:48 AM, Alexandre Belloni wrote: Tony, On 18/08/2015 at 15:11:13 +0530, Keerthy wrote : The series is applicable for all am437x series of processors. It adds clock handling support. Boot tested on am437x-gp-evm. Keerthy (3): ARM: dts: AM437x: Add the internal and external clock nodes for rtc rtc: omap: Add internal clock enabling support rtc: omap: Add external clock enabling support I'm wondering how you want to get those patches merged. I can let you take 2 and 3 through arm-soc but you will miss 4.3. Or I can take 2 and 3 for 4.3 but the documentation will be missing. A gentle ping on this series. Regards, Keerthy -- 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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
Hi,
* Guenter Roeck li...@roeck-us.net [150817 13:48]:
Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes
the call to smsc911x_probe_config() unconditional, and no longer fails if
there is no device node. device_get_phy_mode() is called unconditionally,
and if there is no phy node configured returns an error code. This error
code is assigned to phy_interface, and interpreted elsewhere in the code
as valid phy mode. This in turn causes qemu to crash when running a
variant of realview_pb_defconfig.
qemu: hardware error: lan9118_read: Bad reg 0x86
Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT)
Cc: Jeremy Linton jeremy.lin...@arm.com
Cc Graeme Gregory graeme.greg...@linaro.org
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/smsc/smsc911x.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c
b/drivers/net/ethernet/smsc/smsc911x.c
index 0f21aa3bb537..34f97684506b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops
= {
static int smsc911x_probe_config(struct smsc911x_platform_config *config,
struct device *dev)
{
+ int phy_interface;
u32 width = 0;
if (!dev)
return -ENODEV;
- config-phy_interface = device_get_phy_mode(dev);
+ phy_interface = device_get_phy_mode(dev);
+ if (phy_interface 0)
+ return phy_interface;
+
+ config-phy_interface = phy_interface;
device_get_mac_address(dev, config-mac, ETH_ALEN);
Looks like this change makes at least omap boards using smsc911x
fail with -22 for me in Linux next.
Do any of the the device tree configured smsc911x devices actually
have a phy configured?
Regards,
Tony
--
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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
Hi Tony, On 08/26/2015 01:16 PM, Tony Lindgren wrote: [ ... ] We may need two separate patches, one to fix up device_property_read_u32() to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error from device_get_phy_mode(), and to bail out if device_property_read_u32() returns -ENXIO. I guess the device_property_read_u32() change needs to be discussed separately.. So probably best to fix up the regression to smsc911x first. Not sure myself. Jeremy has a point - we don't really know for sure how safe it is to check for -ENODATA (in addition to -ENXIO). Also, fixing device_property_read_u32() turned out to be much easier than I thought. The simpler alternative would be to check the return value from device_property_read_u32() for both -ENXIO and -ENODATA. This would make the code independent of the necessary core changes (which may take a while). I tested this variant, and it works, at least for the non-DT case. Does this make sense ? Yeh I think that would allow fixing up the smsc911x regression while discussing the device_property_read_u32() change. Got a test patch for me to try? You should have two by now to choose from. Guenter -- 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 -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
* Guenter Roeck li...@roeck-us.net [150826 13:58]: Hi Tony, On 08/26/2015 01:16 PM, Tony Lindgren wrote: [ ... ] We may need two separate patches, one to fix up device_property_read_u32() to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error from device_get_phy_mode(), and to bail out if device_property_read_u32() returns -ENXIO. I guess the device_property_read_u32() change needs to be discussed separately.. So probably best to fix up the regression to smsc911x first. Not sure myself. Jeremy has a point - we don't really know for sure how safe it is to check for -ENODATA (in addition to -ENXIO). Also, fixing device_property_read_u32() turned out to be much easier than I thought. The simpler alternative would be to check the return value from device_property_read_u32() for both -ENXIO and -ENODATA. This would make the code independent of the necessary core changes (which may take a while). I tested this variant, and it works, at least for the non-DT case. Does this make sense ? Yeh I think that would allow fixing up the smsc911x regression while discussing the device_property_read_u32() change. Got a test patch for me to try? You should have two by now to choose from. Acked the second version thanks :) Tony -- 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: CONFIG_DEBUG_SHIRQ and PM
On 26 August 2015 at 17:24, Felipe Balbi ba...@ti.com wrote:
[..]
static irqreturn_t tw68_irq(int irq, void *dev_id)
{
struct tw68_dev *dev = dev_id;
u32 status, orig;
int loop;
status = orig = tw_readl(TW68_INTSTAT) dev-pci_irqmask;
Now try to read that register when your clock is gated. That's the
problem I'm talking about. Everything about the handler is functioning
correctly; however clocks are gated in -remove() and free_irq() is
only called *AFTER* -remove() has returned.
Yeah, it's pretty clear you are talking about clocks here. That's
why I said read won't stall in the next paragraph.
[etc]
}
The IRQ handler accesses the device struct and then
reads through PCI. So if you use devm_request_irq
you need to make sure the device struct is still allocated
after remove(), and the PCI read won't stall or crash.
dude, that's not the problem I'm talking about. I still have my
private_data around, what I don't have is:
__
__ ____| | ___ ___| | __
/ _` | / __| |/ _ \ / __| |/ /
| (_| | | (__| | (_) | (__|
\__,_| \___|_|\___/ \___|_|\_\
Yes, *you* may have your private data around and have a clock gated,
others (the tw68 for instance) may have its region released and unmapped.
And yet others may have $whatever resource released in the
remove() and assume it's available in the IRQ handler.
I honestly can't think why using request_irq / free_irq to solve this
is a workaround.
Interestingly, tw68 uses devm_request_irq with IRQF_SHARED :-)
Still, I don't think that's a good idea, since it relies on
the IRQ being freed *before* the device struct.
that's not an issue at all. If you're using devm_request_irq() you're
likely using devm_kzalloc() for the device struct anyway. Also, you
called devm_kzalloc() before devm_request_irq() so IRQ *will* be freed
before your private data; there's nothing wrong there.
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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 2/2] gpiolib: add description for gpio irqchip fields in struct gpio_chip
On Mon, Aug 17, 2015 at 2:35 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: Add missed description for GPIO irqchip fields in struct gpio_chip. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- Changes in v2: - New patch. Patch applied. Thanks for writing docs, it's very helpful. Yours, Linus Walleij -- 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 3/3] serial: 8250_omap: try to avoid IER_RDI with DMA
On Friday 14 August 2015 09:31 PM, Sebastian Andrzej Siewior wrote: It has been observed on DRA7-evm that the UART triggers the interrupt and reading IIR says IIR_NO_INT. It seems that we receive data via RX-DMA but the interrupt is triggered anyway. I have hardly observed it on AM335x and not in *that* quantity. On DRA7-evm with continuous transfers at 3MBaud and CPU running at 1.5Ghz it is possible that the IRQ-core will close the UART interrupt after some time with nobody cared. I've seen that by not enabling IER_RDI those spurious interrupts are not triggered. Also it seems that DMA and RDI cause the timeout interrupt which does not allow RX-DMA to be scheduled even if the FIFO reached the programmed RX threshold. However without RDI we don't get a notification if we have less than RX threshold bytes in the FIFO. This is where we have the rx_dma_wd timer. After programming the RX-DMA transfer wait HZ / 4 or 250ms for it to complete. If it does not complete in that time span we cancel the DMA transfer and enable RDI. RDI will trigger an UART interrupt in case we have bytes in the FIFO. Once we read bytes manually from the FIFO we enable RX-DMA again (without RDI) with the same 250ms timeout. One downside with this approach is that latency sensitive protocols that transfer less than 48 bytes will have to wait 250ms to complete. I guess because of this reason, wlink8 bluetooth connected to TI's DRA7 EVM failed to probe with this patch included. At the least, looks like this patch needs some tuning. Is there maybe a user interface where one could set small or bulk transfers? Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Thanks, Sekhar -- 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 v4 00/13] USB: OTG/DRD Core functionality
On Mon, Aug 24, 2015 at 04:21:11PM +0300, Roger Quadros wrote: Hi, This series centralizes OTG/Dual-role functionality in the kernel. As of now I've got Dual-role functionality working pretty reliably on dra7-evm and am437x-gp-evm. DWC3 controller and platform related patches will be sent separately. I will review this patch set after you send the above, it will be more clear if there are users for code. Peter Series is based on Greg's usb-next tree. Changelog: - v4: - Added DT support for tying otg-controller to host and gadget controllers. For DT we no longer have the constraint that OTG controller needs to be parent of host and gadget. They can be tied together using the otg-controller property. - Relax the requirement for DT case that otg controller must register before host/gadget. We maintain a wait list of host/gadget devices waiting on the otg controller. - Use a single struct usb_otg for otg data. - Don't override host/gadget start/stop APIs. Let the controller drivers do what they want as they know best. Helper API is provided for controller start/stop that controller driver can use. - Introduce struct usb_otg_config to pass the otg capabilities, otg ops and otg timer timeouts during otg controller registration. - rebased on Greg's usb.git/usb-next v3: - all otg related definations now in otg.h - single kernel config USB_OTG to enable OTG core and FSM. - resolved symbol dependency issues. - use dev_vdbg instead of VDBG() in usb-otg-fsm.c - rebased on v4.2-rc1 v2: - Use add/remove_hcd() instead of start/stop_hcd() to enable/disable the host controller - added dual-role-device (DRD) state machine which is a much simpler mode of operation when compared to OTG. Here we don't support fancy OTG features like HNP, SRP, on the fly role-swap. The mode of operation is determined based on ID pin (cable type) and the role doesn't change till the cable type changes. Why?: Most of the OTG drivers have been dealing with the OTG state machine themselves and there is a scope for code re-use. This has been partly addressed by the usb/common/usb-otg-fsm.c but it still leaves the instantiation of the state machine and OTG timers to the controller drivers. We re-use usb-otg-fsm.c but go one step further by instantiating the state machine and timers thus making it easier for drivers to implement OTG functionality. Newer OTG cores support standard host interface (e.g. xHCI) so host and gadget functionality are no longer closely knit like older cores. There needs to be a way to co-ordinate the operation of the host and gadget in OTG mode. i.e. to stop and start them from a central location. This central location should be the USB OTG core. Host and gadget controllers might be sharing resources and can't be always running. One has to be stopped for the other to run. This can't be done as of now and can be done from the OTG core. What?: - The OTG core instantiates the OTG/DRD Finite State Machine per OTG controller and manages starting/stopping the host and gadget controllers based on the bus state. It provides APIs for the following - Registering an OTG capable controller struct otg_fsm *usb_otg_register(struct device *dev, struct usb_otg_config *config); int usb_otg_unregister(struct device *dev); - Registering Host controllers to OTG core (used by hcd-core) int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags, struct otg_hcd_ops *ops); int usb_otg_unregister_hcd(struct usb_hcd *hcd); - Registering Gadget controllers to OTG core (used by udc-core) int usb_otg_register_gadget(struct usb_gadget *gadget, struct otg_gadget_ops *ops); int usb_otg_unregister_gadget(struct usb_gadget *gadget); - Providing inputs to and kicking the OTG state machine void usb_otg_sync_inputs(struct otg_fsm *fsm); int usb_otg_kick_fsm(struct device *hcd_gcd_device); - Getting controller device structure from OTG state machine instance struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm); 'struct otg_fsm' is the interface to the OTG state machine. It contains inputs to the fsm, status of the fsm and operations for the OTG controller driver. - Helper APIs for starting/stopping host/gadget controllers int usb_otg_start_host(struct otg_fsm *fsm, int on); int usb_otg_start_gadget(struct otg_fsm *fsm, int on); Usage model: --- - The OTG core needs to know what host and gadget controllers are linked to the OTG controller. For DT boots we can provide that information by adding otg-controller property to the host and gadget controller nodes that points to the right otg controller. For legacy boot we assume that OTG controller is the parent of the host and gadget controllers. For DT if otg-controller property is not present then parent child
Re: [PATCH 1/7] gpio: omap: remove wrong irq_domain_remove usage in probe
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: The bank-chip.irqdomain is uninitialized at the moment when irq_domain_remove() is called, so remove this call. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Obviously correct, patch applied. Yours, Linus Walleij -- 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 4/7] gpio: omap: protect regs access in omap_gpio_irq_handler
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: The access to HW registers has to be be protected in omap_gpio_irq_handler(), as it may race with code executed on another CPUs. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Patch applied. Yours, Linus Walleij -- 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/7] gpio: omap: fix clk_prepare/unprepare usage
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: As per CCF documentation (clk.txt) the clk_prepare/unprepare APIs are not allowed in atomic context. But now OMAP GPIO driver uses them while applying debounce settings and as part of PM runtime irqsafe operations: - omap_gpio_debounce() is holding the lock with IRQs off. + omap2_set_gpio_debounce() + clk_prepare_enable() + clk_prepare() this one might sleep. - pm_runtime_get_sync() is holding the lock with IRQs off + omap_gpio_runtime_suspend() + raw_spin_lock_irqsave() + omap_gpio_dbck_disable() + clk_disable_unprepare() Hence, fix it by moeving dbclk prepare/unprepare in OMAP GPIO omap_gpio_probe/omap_gpio_remove. Also, while here, ensure that debounce functionality is disabled if clk_get() failed, because otherwise kernel will carsh in omap2_set_gpio_debounce(). Reported-by: Sebastian Andrzej Siewior bige...@linutronix.de Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Patch applied. Yours, Linus Walleij -- 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 3/7] gpio: omap: fix omap2_set_gpio_debounce
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: According to TRMs: Required input line stable = (the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) × 31, where the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME bit field is from 0 to 255. But now omap2_set_gpio_debounce() will calculate debounce time and behave incorrectly in the following cases: 1) requested debounce time is !0 and 32 calculated DEBOUNCETIME = 0x1 == 62 us; expected value of DEBOUNCETIME = 0x0 == 31us 2) requested debounce time is 0 calculated DEBOUNCETIME = 0x1 == 62 us; expected: disable debounce and DEBOUNCETIME = 0x0 3) requested debounce time is 32 and 63 calculated DEBOUNCETIME = 0x0 and debounce will be disabled; expected: enable debounce and DEBOUNCETIME = 0x1 == 62 us Hence, rework omap2_set_gpio_debounce() to fix above cases: 1) introduce local variable enable and use it to identify when debounce need to be enabled or disabled. Disable debounce if requested debounce time is 0. 2) use below formula for debounce time calculation: debounce = (DIV_ROUND_UP(debounce, 31) - 1) 0xFF; Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Very hightech. Patch applied. Yours, Linus Walleij -- 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/3] serial: 8250_omap: check how many bytes were injected
On 08/14/2015 12:01 PM, Sebastian Andrzej Siewior wrote: The function tty_insert_flip_string() returns an int and as such it might fail. So the result is that I kindly asked to insert 48 bytes and the function only insterted 32. I have no idea what to do with the remaining 16 so I think dropping them is the only option. I also increase the buf_overrun counter so userpace has a clue that we lost bytes. No objection to the patch but I'm curious whether this is something you've actually observed and under what circumstances. Regards, Peter Hurley Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/tty/serial/8250/8250_omap.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 2ac63c8bd946..933f7ef2c004 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -738,6 +738,7 @@ static void __dma_rx_do_complete(struct uart_8250_port *p, bool error) struct dma_tx_state state; int count; unsigned long flags; + int ret; dma_sync_single_for_cpu(dma-rxchan-device-dev, dma-rx_addr, dma-rx_size, DMA_FROM_DEVICE); @@ -753,8 +754,10 @@ static void __dma_rx_do_complete(struct uart_8250_port *p, bool error) count = dma-rx_size - state.residue; - tty_insert_flip_string(tty_port, dma-rx_buf, count); - p-port.icount.rx += count; + ret = tty_insert_flip_string(tty_port, dma-rx_buf, count); + + p-port.icount.rx += ret; + p-port.icount.buf_overrun += count - ret; unlock: spin_unlock_irqrestore(priv-rx_dma_lock, flags); -- 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 16/16] mmc: host: omap_hsmmc: use mmc_of_parse_voltage to get ocr_avail
Hi Ulf,
On Tuesday 25 August 2015 08:20 PM, Ulf Hansson wrote:
On 3 August 2015 at 14:26, Kishon Vijay Abraham I kis...@ti.com wrote:
From: Roger Quadros rog...@ti.com
For platforms that doesn't have explicit regulator control in MMC,
populate voltage-ranges in MMC device tree node and use
mmc_of_parse_voltage to get ocr_avail
I don't like this.
If we are able to fetch the OCR mask via an external regulator, that
shall be done.
I think the mmc_of_parse_voltage() API and the corresponding DT
binding it parses, should be used for those HW when we don't have an
external regulator to use. For example if the MMC controller itself
somehow controls the voltage levels. Is that really the case for you?
This was actually added to support Galileo platform which doesn't have
regulators modelled. Indeed it would be better to model external
regulators (even if it is always on) and get OCR from the regulator.
I'll drop this patch and re-send the series re-based to your next branch.
Thanks
Kishon
Kind regards
Uffe
Signed-off-by: Roger Quadros rog...@ti.com
Signed-off-by: Lokesh Vutla lokeshvu...@ti.com
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Franklin S Cooper Jr fcoo...@ti.com
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
.../devicetree/bindings/mmc/ti-omap-hsmmc.txt |2 ++
drivers/mmc/host/omap_hsmmc.c |9 -
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index 76bf087..2408e87 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -22,6 +22,8 @@ ti,dual-volt: boolean, supports dual voltage cards
ti,non-removable: non-removable slot (like eMMC)
ti,needs-special-reset: Requires a special softreset sequence
ti,needs-special-hs-handling: HSMMC IP needs special setting for handling
High Speed
+voltage-ranges: Specify the voltage range supported if regulator framework
+isn't enabled.
dmas: List of DMA specifiers with the controller specific format
as described in the generic DMA client binding. A tx and rx
specifier is required.
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 15973f1..d884d8f 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2184,7 +2184,13 @@ static int omap_hsmmc_probe(struct platform_device
*pdev)
goto err_irq;
}
- mmc-ocr_avail = mmc_pdata(host)-ocr_mask;
+ if (!mmc_pdata(host)-ocr_mask) {
+ ret = mmc_of_parse_voltage(pdev-dev.of_node,
mmc-ocr_avail);
+ if (ret)
+ goto err_parse_voltage;
+ } else {
+ mmc-ocr_avail = mmc_pdata(host)-ocr_mask;
+ }
omap_hsmmc_disable_irq(host);
@@ -2224,6 +2230,7 @@ static int omap_hsmmc_probe(struct platform_device
*pdev)
err_slot_name:
mmc_remove_host(mmc);
+err_parse_voltage:
omap_hsmmc_reg_put(host);
err_irq:
device_init_wakeup(pdev-dev, false);
--
1.7.9.5
--
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/3] serial: 8250: move rx_running out of the bitfield
On Wednesday 26 August 2015 06:13 PM, Peter Hurley wrote: On 08/26/2015 05:37 AM, Sekhar Nori wrote: On Friday 14 August 2015 09:31 PM, Sebastian Andrzej Siewior wrote: From: John Ogness john.ogn...@linutronix.de That bitfield is modified by read + or + write operation. If someone sets any of the other two bits it might render the lock useless. While at it, remove other bitfields as well to avoid more such errors. Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: John Ogness john.ogn...@linutronix.de Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Tested with wilink BT module on TI's DRA7 EVM. Tested-by: Sekhar Nori nsek...@ti.com Already in Greg's tty-next tree (and 4.3-rc1 pull request), Sekhar. Oops, no problem. Did not notice that. Thanks, Sekhar -- 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/3] serial: 8250: move rx_running out of the bitfield
On 08/26/2015 05:37 AM, Sekhar Nori wrote: On Friday 14 August 2015 09:31 PM, Sebastian Andrzej Siewior wrote: From: John Ogness john.ogn...@linutronix.de That bitfield is modified by read + or + write operation. If someone sets any of the other two bits it might render the lock useless. While at it, remove other bitfields as well to avoid more such errors. Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: John Ogness john.ogn...@linutronix.de Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Tested with wilink BT module on TI's DRA7 EVM. Tested-by: Sekhar Nori nsek...@ti.com Already in Greg's tty-next tree (and 4.3-rc1 pull request), Sekhar. Regards, Peter Hurley -- 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: switch to use platform_get_irq
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: Switch OMAP GPIO driver to use platform_get_irq(), because it is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ, ..) for requesting IRQ resources any more, as they can be not ready yet in case of DT-boot. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Sane handling of deffred probe. Patch applied. Yours, Linus Walleij -- 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/7] gpio: omap: fixes and improvements
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: This patch series contains set of trivial fixes and improvements, and also patches which fixes wrong APIs usage in atomic context as for -RT as for non-RT kernel. The final goal of this series is to make TI OMAP GPIO driver compatible with -RT kernel as much as possible. Patch 1-4: trivial fixes and improvements Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet I've applied patches 1-5 with Santosh's ACK and Tony's Tested-by. Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime can't be used in atimic context on -RT. Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq handler instead of chained IRQ handler. This way OMAP GPIO driver will be compatible with RT kernel where it will be forced thread IRQ handler while in non-RT kernel it still will be executed in HW IRQ context. Waiting for more feedback here. Yours, Linus Walleij -- 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/3] serial: 8250_omap: check how many bytes were injected
On Friday 14 August 2015 09:31 PM, Sebastian Andrzej Siewior wrote: The function tty_insert_flip_string() returns an int and as such it might fail. So the result is that I kindly asked to insert 48 bytes and the function only insterted 32. I have no idea what to do with the remaining 16 so I think dropping them is the only option. I also increase the buf_overrun counter so userpace has a clue that we lost bytes. Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Tested with wilink BT module on TI's DRA7 EVM. Tested-by: Sekhar Nori nsek...@ti.com Thanks, Sekhar -- 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/3] serial: 8250: move rx_running out of the bitfield
On Friday 14 August 2015 09:31 PM, Sebastian Andrzej Siewior wrote: From: John Ogness john.ogn...@linutronix.de That bitfield is modified by read + or + write operation. If someone sets any of the other two bits it might render the lock useless. While at it, remove other bitfields as well to avoid more such errors. Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: John Ogness john.ogn...@linutronix.de Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Tested with wilink BT module on TI's DRA7 EVM. Tested-by: Sekhar Nori nsek...@ti.com Thanks, Sekhar -- 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: omapdss: Division by zero in kernel
On Friday 21 August 2015 12:17:41 Tomi Valkeinen wrote: On 21/08/15 11:48, Pali Rohár wrote: On Friday 21 August 2015 11:42:14 Tomi Valkeinen wrote: On 24/07/15 19:03, Pali Rohár wrote: Hello, when on N900 (real HW or qemu) I run this command / # echo 0 /sys/devices/platform/omapdss/overlay0/enabled echo 0 /sys/class/graphics/fb0/size then kernel crash with this error message / # [ 29.904113] Division by zero in kernel. Hi! Thanks for explaining. The problem is that fb console uses the kernel mmapped framebuffer, but omapfb is not aware of the fb console. So the above commands free the framebuffer, as omapfb thinks no one is using it, and then fb console tries to touch the fb. What about refusing those calls from fb console? So fb console will not know about this problem and omapfb will just ignore drawn functions? Hmm, I'm not sure I understand what you mean... omapfb is not drawing anything, fbcon is doing the drawing independently to the fb. And the fb suddenly disappears without fbcon realizing that. omapfb tracks mmaps from userspace, and refuses to free a fb it it's mmapped. I don't know how to fix it straight away. Maybe there's a way for omapfb to check if the fbcon uses the fb in question, and if so, refuses to release/resize the memory. Tomi Maemo userspace (on Nokia N900) uses above commands to initialize graphic and Xserver. So it would be nice if disabling framebuffer would work even if fbcon.ko is loaded (or compiled directly into zImage). Ok. And N900 has fbcon enabled? I wonder how it survives... Depends on compiled kernel. Original stock Nokia kernel 2.6.28 has it disabled, but when I recompiled it with fbcon (either static linked into zImage or external fbcon.ko) it works and I do not see any problem. So I think it survives... fbcon can be unbound from userspace with something like: echo 0 /sys/class/vtconsole/vtcon1/bind After that I think the memory can be freed. But obviously the kernel should not crash here, no question about that. Tomi Maybe just adding that test for zero to prevent division by zero? -- Pali Rohár pali.ro...@gmail.com -- 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] of: device: fix NULL pointer dereference on driver removal
On Tue, Aug 25, 2015 at 2:34 PM, Felipe Balbi ba...@ti.com wrote: If we don't insert resources into the resource tree, calls to of_platform_depopulate() will end up in NULL pointer dereferences because the resource parent will be set to NULL even though we still have more resources to go through. Long standing problem. A fix is in -next now and will go into 4.3 (plus stable): commit 11c63e4718acad3d8f04601c80fddd4b3d1455b1 Author: Grant Likely grant.lik...@linaro.org Date: Sun Jun 7 15:20:11 2015 +0100 drivercore: Fix unregistration path of platform devices Without this patch, the result is as follows: [ 28.238158] Unable to handle kernel NULL pointer dereference at virtual address 0008 [ 28.246646] pgd = ed3d8000 [ 28.249480] [0008] *pgd= [ 28.253247] Internal error: Oops: 5 [#1] SMP ARM [ 28.258072] Modules linked in: input_leds hid_generic usbkbd usbhid xhci_plat_hcd xhci_hcd usbcore joydev dwc3 udc_core usb_common m25p80 evdev spi_nor omapfb cfbfillrect cfbimg blt cpufreq_dt cfbcopyarea thermal_sys snd_soc_simple_card leds_gpio matrix_keypad pwm_bl hwmon led_class matrix_keymap panel_dpi snd_soc_tlv320aic3x snd_soc_davinci_mcasp snd_soc_edma snd_soc_omap snd_soc_core omapdss snd_compress snd_pcm_dmaengine snd_pcm dwc3_omap(-) extcon snd_timer pwm_tiecap snd lis3lv02d_i2c lis3lv02d soundcore input_polldev rtc_omap spi_ti_qspi tps65218_pwrbutton omap_wdt phy_omap_usb2 autofs4 [ 28.313186] CPU: 0 PID: 289 Comm: modprobe Not tainted 4.2.0-rc7-next-20150824-2-g5681a109a938-dirty #1093 [ 28.323643] Hardware name: Generic AM43 (Flattened Device Tree) [ 28.329836] task: ed39d180 ti: ed076000 task.ti: ed076000 [ 28.335496] PC is at __release_resource+0x30/0x13c [ 28.340501] LR is at __release_resource+0x24/0x13c [ 28.345501] pc : [c00439e0]lr : [c00439d4]psr: 600d0013 [ 28.345501] sp : ed077e98 ip : 0007 fp : befb3e14 [ 28.357487] r10: r9 : ed076000 r8 : c000f724 [ 28.362941] r7 : r6 : ee6f9800 r5 : ed268d40 r4 : c094679c [ 28.369755] r3 : r2 : 00f4 r1 : c0648b34 r0 : 0045 [ 28.376560] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [ 28.384008] Control: 10c5387d Table: ad3d8059 DAC: 0015 [ 28.390005] Process modprobe (pid: 289, stack limit = 0xed076218) [ 28.396375] Stack: (0xed077e98 to 0xed078000) [ 28.400924] 7e80: ed268d40 [ 28.409455] 7ea0: befb3e14 c0640838 0001 c094679c ed268d40 ee6f9800 0081 c0043b1c [ 28.417996] 7ec0: 001c 0001 ee6f9800 c03e2674 ee6f9800 c04d3f20 c03e26ac [ 28.426537] 7ee0: ee6f9810 c04d3fac c03dcf10 ee1592c0 ed268cf0 ee170010 ee170010 [ 28.435065] 7f00: ee170044 c04d3f08 ed268ed0 bf093208 ee170010 bf093c94 ee170044 c03e2754 [ 28.443607] 7f20: ee170010 bf093c94 ee170044 c03e095c ee170010 bf093c94 ee170044 c03e116c [ 28.452150] 7f40: bf093c94 7f6232fc 0800 c03e04e4 bf093d00 c00c8a80 33637764 [ 28.460703] 7f60: 616d6f5f b6f70070 ed39d180 ed076000 befb3e14 c005be74 [ 28.469241] 7f80: ed076000 7f6232c0 7f6232c0 0001 f67c 7f6232c0 7f6232c0 [ 28.477783] 7fa0: 0001 c000f540 7f6232c0 7f6232c0 7f6232fc 0800 66d19c00 [ 28.486341] 7fc0: 7f6232c0 7f6232c0 0001 0081 0001 7f6232c0 befb3e14 [ 28.494903] 7fe0: b6f1c2e1 befb2a3c 7f60638f b6f1c2e6 800d0030 7f6232fc [ 28.503476] [c00439e0] (__release_resource) from [c0043b1c] (release_resource+0x30/0x54) [ 28.512299] [c0043b1c] (release_resource) from [c03e2674] (platform_device_del+0x70/0x9c) [ 28.521218] [c03e2674] (platform_device_del) from [c03e26ac] (platform_device_unregister+0xc/0x20) [ 28.530962] [c03e26ac] (platform_device_unregister) from [c04d3fac] (of_platform_device_destroy+0x8c/0x98) [ 28.541425] [c04d3fac] (of_platform_device_destroy) from [c03dcf10] (device_for_each_child+0x50/0x7c) [ 28.551430] [c03dcf10] (device_for_each_child) from [c04d3f08] (of_platform_depopulate+0x2c/0x44) [ 28.561101] [c04d3f08] (of_platform_depopulate) from [bf093208] (dwc3_omap_remove+0x3c/0x5c [dwc3_omap]) [ 28.571390] [bf093208] (dwc3_omap_remove [dwc3_omap]) from [c03e2754] (platform_drv_remove+0x18/0x30) [ 28.581387] [c03e2754] (platform_drv_remove) from [c03e095c] (__device_release_driver+0x88/0x114) [ 28.591023] [c03e095c] (__device_release_driver) from [c03e116c] (driver_detach+0xb4/0xb8) [ 28.600014] [c03e116c] (driver_detach) from [c03e04e4] (bus_remove_driver+0x4c/0xa0) [ 28.608485] [c03e04e4] (bus_remove_driver) from [c00c8a80] (SyS_delete_module+0x11c/0x1e4) [ 28.617518] [c00c8a80] (SyS_delete_module) from [c000f540] (ret_fast_syscall+0x0/0x54) [ 28.626172] Code: eb0354bf e5957010 e3a020f4 e59f10f0 (e5973008) [ 28.632722] ---[ end trace d2a21fc5d73a2dfd ]--- Cc:
Re: [PATCH] of: device: fix NULL pointer dereference on driver removal
On Wed, Aug 26, 2015 at 08:14:36AM -0500, Rob Herring wrote: On Tue, Aug 25, 2015 at 2:34 PM, Felipe Balbi ba...@ti.com wrote: If we don't insert resources into the resource tree, calls to of_platform_depopulate() will end up in NULL pointer dereferences because the resource parent will be set to NULL even though we still have more resources to go through. Long standing problem. A fix is in -next now and will go into 4.3 (plus stable): commit 11c63e4718acad3d8f04601c80fddd4b3d1455b1 Author: Grant Likely grant.lik...@linaro.org Date: Sun Jun 7 15:20:11 2015 +0100 drivercore: Fix unregistration path of platform devices that commit works, but too late to add a Tested-by :-) -- balbi signature.asc Description: Digital signature
[PATCH 1/2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
Reconfigure and restart audio when display is enabled, if audio
playback was active before. The audio configuration is stored when is
is successfully applied and a boolean is set when playback has started
and unset when stopped. This data is used to reconfigure the audio
when display is re-enabled. Abort audio playback if reconfiguration
fails.
A new spin lock is introduced to in order protect state variables
related to audio playback status. The wp_idlemode protection relies on
PM not touching it after the original initialization. A power
management API for controlling the idlemode would be needed for proper
synchronization.
Signed-off-by: Jyri Sarha jsa...@ti.com
---
drivers/video/fbdev/omap2/dss/hdmi.h | 10 +++-
drivers/video/fbdev/omap2/dss/hdmi4.c | 65 -
drivers/video/fbdev/omap2/dss/hdmi5.c | 89 +--
3 files changed, 137 insertions(+), 27 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h
b/drivers/video/fbdev/omap2/dss/hdmi.h
index e4a32fe..1e45b84 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi.h
+++ b/drivers/video/fbdev/omap2/dss/hdmi.h
@@ -351,12 +351,20 @@ struct omap_hdmi {
struct regulator *vdda_reg;
bool core_enabled;
- bool display_enabled;
struct omap_dss_device output;
struct platform_device *audio_pdev;
void (*audio_abort_cb)(struct device *dev);
+
+ bool audio_configured;
+ struct omap_dss_audio audio_config;
+
+ /* This lock should be taken when any one of the three
+ state variables bellow are touched. */
+ spinlock_t audio_playing_lock;
+ bool audio_playing;
+ bool display_enabled;
int wp_idlemode;
};
diff --git a/drivers/video/fbdev/omap2/dss/hdmi4.c
b/drivers/video/fbdev/omap2/dss/hdmi4.c
index 6d3aa3f..ea1fa64 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi4.c
@@ -324,6 +324,7 @@ static int read_edid(u8 *buf, int len)
static int hdmi_display_enable(struct omap_dss_device *dssdev)
{
struct omap_dss_device *out = hdmi.output;
+ unsigned long flags;
int r = 0;
DSSDBG(ENTER hdmi_display_enable\n);
@@ -342,7 +343,26 @@ static int hdmi_display_enable(struct omap_dss_device
*dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ hdmi4_audio_stop(hdmi.core, hdmi.wp);
+ hdmi_wp_audio_enable(hdmi.wp, false);
+
+ r = hdmi4_audio_config(hdmi.core, hdmi.wp, hdmi.audio_config,
+ hdmi.cfg.timings.pixelclock);
+ if (r) {
+ DSSERR(Error restoring audio configuration: %d, r);
+ hdmi.audio_abort_cb(hdmi.pdev-dev);
+ hdmi.audio_configured = false;
+ }
+ }
+
+ spin_lock_irqsave(hdmi.audio_playing_lock, flags);
+ if (hdmi.audio_configured hdmi.audio_playing) {
+ hdmi_wp_audio_enable(hdmi.wp, true);
+ hdmi4_audio_start(hdmi.core, hdmi.wp);
+ }
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(hdmi.audio_playing_lock, flags);
mutex_unlock(hdmi.lock);
return 0;
@@ -354,17 +374,18 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG(Enter hdmi_display_disable\n);
mutex_lock(hdmi.lock);
- if (hdmi.audio_pdev hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(hdmi.audio_pdev-dev);
+ spin_lock_irqsave(hdmi.audio_playing_lock, flags);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(hdmi.lock);
}
@@ -565,9 +586,14 @@ out:
static int hdmi_audio_shutdown(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
mutex_lock(hd-lock);
hd-audio_abort_cb = NULL;
+ hd-audio_configured = false;
+ spin_lock_irqsave(hd-audio_playing_lock, flags);
+ hd-audio_playing = false;
+ spin_unlock_irqrestore(hd-audio_playing_lock, flags);
mutex_unlock(hd-lock);
return 0;
@@ -576,25 +602,38 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(hd-cfg));
- WARN_ON(!hd-display_enabled);
- hdmi_wp_audio_enable(hd-wp, true);
- hdmi4_audio_start(hd-core, hd-wp);
+ spin_lock_irqsave(hd-audio_playing_lock, flags);
+
+ if (hd-display_enabled) {
+ hdmi_wp_audio_enable(hd-wp, true);
+ hdmi4_audio_start(hd-core, hd-wp);
+ }
+ hd-audio_playing = true;
+
[PATCH 0/2] OMAPDSS: Couple of HDMI audio fixes
The first fix is a hairy one, but I think the locking is fool proof now. It is needed because there is no telling in which order user space starts an audio and video stream playback. If the audio is started first and the video mode is changed when video playback starts the audio setup needs to survive display turning off and back on again. After this patch the audio streams should survive a suspend-resume cycle too. The second one is a trivial work-around to a problem in ALSA constraint resolver code. The two fixes are totally independent and the video and audio side patches applied separately trough their own trees. Jyri Sarha (2): OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled ASoC: omap-hdmi-audio: Set buffer bytes step constraint to 128 drivers/video/fbdev/omap2/dss/hdmi.h | 10 +++- drivers/video/fbdev/omap2/dss/hdmi4.c | 65 - drivers/video/fbdev/omap2/dss/hdmi5.c | 89 +-- sound/soc/omap/omap-hdmi-audio.c | 10 +++- 4 files changed, 146 insertions(+), 28 deletions(-) -- 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
[PATCH 2/2] ASoC: omap-hdmi-audio: Set buffer bytes step constraint to 128
Set buffer bytes step constraint to 128. A matching constraint has
already been set to period size. This helps PCM setup to tolerate ALSA
clients that set the PCM hw params in unusual order.
Signed-off-by: Jyri Sarha jsa...@ti.com
---
sound/soc/omap/omap-hdmi-audio.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-hdmi-audio.c b/sound/soc/omap/omap-hdmi-audio.c
index aeef25c..584b237 100644
--- a/sound/soc/omap/omap-hdmi-audio.c
+++ b/sound/soc/omap/omap-hdmi-audio.c
@@ -81,7 +81,15 @@ static int hdmi_dai_startup(struct snd_pcm_substream
*substream,
ret = snd_pcm_hw_constraint_step(substream-runtime, 0,
SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
if (ret 0) {
- dev_err(dai-dev, could not apply constraint\n);
+ dev_err(dai-dev, Could not apply period constraint: %d\n,
+ ret);
+ return ret;
+ }
+ ret = snd_pcm_hw_constraint_step(substream-runtime, 0,
+SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 128);
+ if (ret 0) {
+ dev_err(dai-dev, Could not apply buffer constraint: %d\n,
+ ret);
return ret;
}
--
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/3] serial: 8250_omap: check how many bytes were injected
On Wednesday 26 August 2015 06:14 PM, Peter Hurley wrote: On 08/14/2015 12:01 PM, Sebastian Andrzej Siewior wrote: The function tty_insert_flip_string() returns an int and as such it might fail. So the result is that I kindly asked to insert 48 bytes and the function only insterted 32. I have no idea what to do with the remaining 16 so I think dropping them is the only option. I also increase the buf_overrun counter so userpace has a clue that we lost bytes. No objection to the patch but I'm curious whether this is something you've actually observed and under what circumstances. This was observed while doing a UART internal loopback test at 3Mbaud on TI's DRA7 EVM. Thanks, Sekhar -- 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: CONFIG_DEBUG_SHIRQ and PM
Hi,
On Wed, Aug 26, 2015 at 05:15:51PM -0300, Ezequiel Garcia wrote:
snip
be prepared to handle it any time, coming from any sources (not only
your device). And CONFIG_DEBUG_SHIRQ does exactly that, in order to
make sure all the drivers passing IRQF_SHARED comply with that rule.
you need to be sure of that with non-shared IRQs anyway.
Not entirely. If your IRQ is not shared, then you usually have a register
to enable or unmask your peripheral interrupts. So the driver is in control
of when it will get interrupts.
If the IRQ is shared, this won't do. This is what I mean by shared IRQs
must be prepared to receive an interrupt any time, in the sense that
the driver has no way of preventing IRQs (because they may be
coming from any source).
right, the problem is much less likely on non-shared lines but the fine
that a line is shared or not is a function of HW integration, not the
e.g. USB Controller, so that knowledge really doesn't fit the driver in
a sense.
We might as well get rid of IRQF_SHARED and assume all lines are
shareable.
In the same sense, shared IRQs handlers need to double-check
the IRQ is coming to the current device by checking some IRQ
status register to see if there's pending work.
you should the status register even on non-shared IRQs to catch spurious
right ?
Also, an IRQ
which isn't shared in SoC A, might become shared in SoC B which uses the
same IP.
So you either avoid using devm_request_irq, or you prepare your handler
accordingly to be ready to handle an interrupt _any time_.
the handler is ready to handle at any time, what isn't correct is the
fact that clocks get gated before IRQ is freed.
There should be no such special case as if your handler is shared,
don't use devm_request_*irq() because if we just disable PM_RUNTIME, it
works as expected anyway.
Yeah, I meant to say: if you use devm_request_irq with IRQF_SHARED
then you _must_ be prepared to get an IRQ *after* your remove() has
been called.
Let's consider this snippet from tw68:
static irqreturn_t tw68_irq(int irq, void *dev_id)
{
struct tw68_dev *dev = dev_id;
u32 status, orig;
int loop;
status = orig = tw_readl(TW68_INTSTAT) dev-pci_irqmask;
Now try to read that register when your clock is gated. That's the
problem I'm talking about. Everything about the handler is functioning
correctly; however clocks are gated in -remove() and free_irq() is
only called *AFTER* -remove() has returned.
[etc]
}
The IRQ handler accesses the device struct and then
reads through PCI. So if you use devm_request_irq
you need to make sure the device struct is still allocated
after remove(), and the PCI read won't stall or crash.
dude, that's not the problem I'm talking about. I still have my
private_data around, what I don't have is:
__
__ ____| | ___ ___| | __
/ _` | / __| |/ _ \ / __| |/ /
| (_| | | (__| | (_) | (__|
\__,_| \___|_|\___/ \___|_|\_\
Interestingly, tw68 uses devm_request_irq with IRQF_SHARED :-)
Still, I don't think that's a good idea, since it relies on
the IRQ being freed *before* the device struct.
that's not an issue at all. If you're using devm_request_irq() you're
likely using devm_kzalloc() for the device struct anyway. Also, you
called devm_kzalloc() before devm_request_irq() so IRQ *will* be freed
before your private data; there's nothing wrong there.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v5 0/3] ARM: AM437X: Add rtc clock handling
On 26/08/2015 at 11:01:27 -0700, Tony Lindgren wrote : * Keerthy a0393...@ti.com [150826 09:54]: Tony, On Saturday 22 August 2015 02:48 AM, Alexandre Belloni wrote: Tony, On 18/08/2015 at 15:11:13 +0530, Keerthy wrote : The series is applicable for all am437x series of processors. It adds clock handling support. Boot tested on am437x-gp-evm. Keerthy (3): ARM: dts: AM437x: Add the internal and external clock nodes for rtc rtc: omap: Add internal clock enabling support rtc: omap: Add external clock enabling support I'm wondering how you want to get those patches merged. I can let you take 2 and 3 through arm-soc but you will miss 4.3. Or I can take 2 and 3 for 4.3 but the documentation will be missing. A gentle ping on this series. Alexandre, it's probably best that you take them all. The dts changes apply against Linux next with fuzz so there should not be any merge conflict. Feel free to add: Acked-by: Tony Lindgren t...@atomide.com So, I've rebased 1/3 on rtc-next to avoid depending on arm-soc. I've pushed everything and hopefully there won't be any issues in linux-next. We are quite close to the merge window so I would be much more confident sending them to Linus if you could test linux-next once the patches land there. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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 v3 0/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
Hi Paul, On Thursday 23 July 2015 06:55 PM, Lokesh Vutla wrote: This series implements lock and unlock functions for RTC and hooks the same to DRA7 and AMx3xx hwmod. This is dependent on the patch https://patchwork.kernel.org/patch/6578281/, which is queued recently by Paul. Gentle ping on this series. Thanks and regards, Lokesh Changes since v2: - Add kerneldoc for omap_hwmod_rtc_lock() function. Lokesh Vutla (3): ARM: hwmod: RTC: Add lock and unlock functions ARM: DRA7: RTC: Add lock and unlock functions ARM: AMx3xx: RTC: Add lock and unlock functions arch/arm/mach-omap2/omap_hwmod.h | 2 + .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_reset.c | 65 ++ 4 files changed, 71 insertions(+) -- 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
